-
Notifications
You must be signed in to change notification settings - Fork 558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mark FaultException as Serializable #4205
Mark FaultException as Serializable #4205
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also be added to the reference assembly. Can you add a simple serialization test to the unit tests for S.SM.Primitives to ensure the serialized payload is the same between NetFx and Core.
Now I've added the attribute to the ref project file System.ServiceModel.Primitives.cs. And I tried to serialize a FaultException instance with XmlSerializer, it doesn't work in .Net Framework nor .Net Core due to error about the base class "Exception" implements IDictionary. I can use BinaryFormater to do serialization for FaultException type but serialized payload comparison with NetFramework is different. Any suggestion? Btw, deserialization from the serialized MemoryStream won't work as there seems has other dependencies not implemented, shall we support that as well? |
XmlSerializer can't serialize IDictionary, but DataContractSerializer can. I couldn't find an instance of MemoryStream on FaultException or Exception. Where are you seeing this? |
I forgot to mention I tried DataContractSerializer, it threw exception with message: System.Runtime.Serialization.InvalidDataContractException : Type 'System.ServiceModel.FaultException+FaultCodeData' cannot be serialized. Consider marking it with the DataContractAttribute attribute, and marking all of its members you want serialized with the DataMemberAttribute attribute. Alternatively, you can ensure that the type is public and has a parameterless constructor - all public members of the type will then be serialized, and no attributes will be required. Regarding the MemoryStream thing, I am sorry it's not clear said, I meant after serialize FaultException instance to a memory stream, I was trying to deserialize it back, but failed to do so. |
The FaultCodeData class also needs the Serializable attribute applied. If you compare the NetFx source with the .NET Core source, you can see that the attribute has been applied in NetFx. |
…test to use DataContractSerializer.
Thanks, @mconnew . I've updated the product and test codes. Serializaing FaultException instance with DataContractSerializer would get same payload as .NetFramework baseline after making following changes:
Does the changes in step1,2,3 make sense to you? Is result in step4 expected? |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Your step 4 is interesting. On Framework, CommunicationException derives from SystemException which sets the default HResult to COR_E_SYSTEM (-2146233087). SystemException didn't exist in Core until netstandard2.0 so we had to derive from Exception instead, which defaults HResult to COR_E_EXCEPTION (-2146233088). This is where you are seeing the change. I'll open an issue around whether we should switch to SystemException as our base type. For now we'll include that change for the tests as it's caused by the base class difference. |
Tests are failing because the Windows machines are running with the Spanish culture "es-ES" and the test expects "en-US". |
@imcarolwang, can you modify the |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@imcarolwang, don't worry about my last comment. I've fixed the build to use the English Windows image. |
For issue #3984 .
@HongGit ,@mconnew , @StephenBonikowsky
I am not sure if mark the FaultException class as Serializable is enough to make the scenario function work, let me know your concerns.